-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Patch repodata to relax HTSlib dependency upper bounds #42895
Conversation
08f3bbb
to
88d74f4
Compare
By patching repodata htslib dependencies, we no longer need to pin htslib. See bioconda/bioconda-recipes#42895.
Upstream HTSlib is careful to maintain binary compatibility between soversion bumps. Note though that those bumps are not correlated with e.g. major version number bumps, as that would reflect a break in source compatibility not binary compatibility. Hence building other bioconda packages that use htslib produces a tight bound on a single x.x htslib version, but we can use repodata patching to widen the bounds on previously build packages, due to this forward compatibility. Doing so will mean bioconda no longer needs to pin HTSlib, as it will be less vital to ensure all dependent packages are built against the exact same version of htslib. In turn, this will simplify packaging htslib/samtools/bcftools updates.
ddaf168
to
9274255
Compare
@jmarshall This looks great and I completely agree that this should make things easier in the future. Thanks a ton! |
By patching repodata htslib dependencies, we no longer need to pin htslib. See bioconda/bioconda-recipes#42895.
What about the bioconda-recipes/recipes/htslib/meta.yaml Lines 9 to 10 in d40dcb5
|
Loosening the (If you think semver would miraculously solve this problem, reflect on the fourth paragraph in the initial comment above and the fact that semver does not distinguish between source code compatibility and ABI compatibility.) |
@jmarshall thanks for the explanation! That makes sense
Yes, I recall that discussion on Twitter. Reading your other posts helped solidify my understanding I think part of my confusion yesterday was not understanding the arguments to bioconda-recipes/recipes/bioconda-repodata-patches/gen_patch_json.py Lines 168 to 172 in a372835
translates to:
Thus a recipe built against htslib 1.10 would be edited to have a pin of Now let's say that hypothetically in the future htslib 1.20 introduces a new SO version, and then releases 1.21, 1.22, and 1.23 are compatible. My guess would be that we would add an additional # future HTSlib versions are binary compatible until they bump their soversion
if has_dep(record, 'htslib'):
# skip deps prior to 1.10, which was the first with soversion 3
# TODO adjust replacement (exclusive) upper bound with each new compatible HTSlib
_pin_looser(fn, record, 'htslib', min_lower_bound='1.10', upper_bound='1.20')
# htslib 1.20 bumped the soversion
_pin_looser(fn, record, 'htslib', min_lower_bound='1.20', upper_bound='1.24') Do I understand the plan correctly? |
Bioconda-repodata-patches's scripts are fairly niche and undocumented. You have translated the existing code correctly. A program built against htslib-1.15 will be compatible until htslib bumps its soversion (unless they make a mistake!), but as it may well use functions newly introduced in htslib-1.15 it won't in general be compatible with earlier versions such as 1.14. Thus it's mostly not useful to reduce the lower pin — but if it was useful for something such functionality could be added to gen_patch_json.py. Adding another |
Somewhat similarly to PR #40675 for libdeflate, this PR recommends patching htslib dependencies to extend version ranges instead of pinning htslib in bioconda. Pinning htslib causes deployment difficulties each time there is an upstream htslib/samtools/bcftools joint release; e.g. #42167 and #42168 have still not been merged six weeks after bcftools and samtools 1.18 were released, because the htslib pinning still has not been updated (cf bioconda/bioconda-utils#915).
The reason for pinning htslib is that it is a common dependency and if several tools using it are all going to be installed in the same environment they all need to be using (and hence built against) the same htslib version.
If we patch repodata to loosen these htslib dependencies, this need will go away. Then we can simplify updates by removing the pinning of htslib (which is now bioconda/bioconda-utils#917).
Upstream HTSlib is careful to maintain binary compatibility between soversion bumps. Note though that those bumps are not correlated with e.g. major version number bumps, as the HTSlib maintainers would use that to reflect a break in source compatibility not binary compatibility.
Hence building other bioconda packages that use htslib produces a tight bound on a single
x.x
htslib version, but we can use repodata patching to widen the bounds on previously build packages, due to this forward compatibility. Doing so will mean bioconda no longer needs to pin HTSlib, as it will be less vital to ensure all dependent packages are built against the exact same version of htslib. In turn, this will simplify packaging htslib/samtools/bcftools updates.Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add label
command.@bioconda/core
in a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports
(see here for the rationale and comprehensive explanation).Add a
run_exports
section like this:with
...
being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}
while replacing
"myrecipe"
with eithername
if aname|lower
variable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update
@BiocondaBot please add label
please review & merge
label.@BiocondaBot please fetch artifacts
You can use this to test packages locally.
Note that the
@BiocondaBot please merge
command is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>
and will automatically re-post them to notify the addressed<team>
.